Log structured peer, channel and hash fields #4217

joostjager · 2025-11-11T12:52:28Z

This PR improves channel ID logging consistency and clarity.

Removes redundant channel ID text from log messages where the ID is
already included as a structured field. This reduces duplication and makes
it easier to filter logs by channel.
Updates the test logger to display the structured channel ID instead of
relying on message text. The logged ID is truncated for readability.
Also log peer id and payment hash as structured fields. Log message de-duplication is left for a follow up.

Together, these changes standardize how channel IDs appear in logs and make
them easier to parse and analyze.

Example output:

node 0 TRACE [lightning::ln::channel:5253]             p:0355f8 ch:ae3367 h:544e62 Building commitment transaction number 281474976710654 (really 1 xor 53018734980789) for channel ae3367da2c13bc1ceb86bf56418f62828f7ce9d6bfb15a46af5ba1f1ed8b124f for remote, generated by us with fee 253...
node 0 TRACE [lightning::ln::channel:5299]             p:0355f8 ch:ae3367 h:544e62    ...including outbound LocalAnnounced HTLC 0 (hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3) with value 10000
node 0 TRACE [lightning::sign::tx_builder:362]         p:0355f8 ch:ae3367 h:544e62    ...trimming outbound HTLC with value 10sat, hash 544e62cee8033709e389e5b2755343d0d0fa8c4850215cfb6331717e80d1aea3, due to dust limit 354
node 0 TRACE [lightning::sign::tx_builder:435]         p:0355f8 ch:ae3367 h:544e62    ...including to_local output with value 99796
node 0 TRACE [lightning::chain::chainmonitor:1407]     p:0355f8 ch:ae3367          Updating ChannelMonitor to id 1
node 0 INFO  [lightning::chain::channelmonitor:4226]   p:0355f8 ch:ae3367          Applying update, bringing update_id from 0 to 1 with 1 change(s).
node 0 TRACE [lightning::chain::channelmonitor:4289]   p:0355f8 ch:ae3367          Updating ChannelMonitor with latest counterparty commitment transaction info
node 0 DEBUG [lightning::chain::chainmonitor:1451]     p:0355f8 ch:ae3367          Persistence of ChannelMonitorUpdate id 1 completed

ldk-reviews-bot · 2025-11-11T12:52:31Z

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

TheBlueMatt · 2025-11-11T13:47:50Z

lightning/src/util/test_utils.rs

-		let s = format!("{:<55} {}", context, record.args);
+		let chan_id = record.channel_id.map(|id| format!("{}", id));
+		let chan_str = chan_id.as_deref().and_then(|s| s.get(..6)).unwrap_or("");
+		let s = format!("{:<55} {:<6} {}", context, chan_str, record.args);


Rather than just updating the test logger, let's instead maybe add a Display implementation for Record that includes all the relevant information? After we do that we'll need to update the one in fuzzing and IMO the docs on the logger to make clear that we intend to not include redundant information in the human-readable part of the log record (and that they absolutely need to include it if they want useful logs).

Display added and docs updated.

codecov · 2025-11-12T09:27:16Z

Codecov Report

❌ Patch coverage is 91.54229% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.36%. Comparing base (d4828c0) to head (e069743).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/logger.rs	80.00%	0 Missing and 9 partials ⚠️
lightning/src/ln/channelmanager.rs	84.31%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4217      +/-   ##
==========================================
- Coverage   89.36%   89.36%   -0.01%     
==========================================
  Files         180      180              
  Lines      138369   138283      -86     
  Branches   138369   138283      -86     
==========================================
- Hits       123652   123574      -78     
+ Misses      12114    12103      -11     
- Partials     2603     2606       +3

Flag	Coverage Δ
fuzzing	`35.93% <54.30%> (+0.05%)`	⬆️
tests	`88.72% <91.54%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

TheBlueMatt · 2025-11-12T13:48:09Z

fuzz/src/onion_message.rs

 		&self, _sender: PublicKey, _peers: Vec<PublicKey>, destination: Destination,
 	) -> Result<OnionMessagePath, ()> {
-		Ok(OnionMessagePath { intermediate_nodes: vec![], destination, first_node_addresses: vec![] })
+		Ok(OnionMessagePath {


wtf rustfmt? This wasn't even changed 🤦

lightning/src/util/logger.rs

TheBlueMatt

LGTM, needs rebase though.

lightning/src/util/logger.rs

As a preparatory step for unifying test log output.

Makes sure that 4 and 5 character log levels line up.

Now that we have Display implemented on Record, the various test logger implementations can make use of that.

joostjager · 2025-11-14T10:50:42Z

Rebased

joostjager · 2025-11-14T15:09:18Z

Named SubstringFormatter struct: https://github.com/lightningdevkit/rust-lightning/compare/2febae0fb29e9e72294d8b31b4e6a335270ac0db..e76d5643a729f9f854796218086235f6a2e0794b

valentinewallace

I would prefer putting the peer and payment hash at the end of the log, at least in test output. I work on a laptop somewhat regularly and this change pushes the actual log content very far to the side, particularly since the payment_hash field doesn't seem to be set the vast majority of the time.

We could also maybe do this in addition:

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 643c1825f..a0cd881f1 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -4360,7 +4360,7 @@ pub fn create_chanmon_cfgs_with_legacy_keys(
                let tx_broadcaster = test_utils::TestBroadcaster::new(Network::Testnet);
                let fee_estimator = test_utils::TestFeeEstimator::new(253);
                let chain_source = test_utils::TestChainSource::new(Network::Testnet);
-               let logger = test_utils::TestLogger::with_id(format!("node {}", i));
+               let logger = test_utils::TestLogger::with_id(format!("n{}", i));
                let persister = test_utils::TestPersister::new();
                let seed = [i as u8; 32];
                let keys_manager = if predefined_keys_ids.is_some() {
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 5f408a9e8..d414b980f 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -1698,7 +1698,7 @@ impl TestLogger {

 impl Logger for TestLogger {
        fn log(&self, record: Record) {
-               let s = format!("{:<6} {}", self.id, record);
+               let s = format!("{:<2} {}", self.id, record);
                #[cfg(ldk_bench)]
                {
                        // When benchmarking, we don't actually want to print logs, but we do want to format

lightning/src/ln/channelmanager.rs

valentinewallace · 2025-11-14T16:29:51Z

lightning/src/ln/invoice_utils.rs

 					entry.insert(channel);
 				} else {
 					log_trace!(logger,
-						"Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",


The word "over" got deleted, may need to tweak the log slightly to keep the original meaning as well

Reverted these two log lines. As they are making a comparison, it may be a bit weird to rely on the structured field only.

joostjager · 2025-11-14T18:19:37Z

We could also maybe do this in addition:

In some tests, the nodes are called 'alice' and 'bob', so we'd need to change that to 'a' and 'b' perhaps? Maybe just one character is also sufficient. Starting everyline with n is probably unnecessary?

Previously, log messages often included the channel ID both in the message text and in the structured `channel_id` field. This led to redundant information and made it difficult to quickly identify which log lines pertain to a given channel, as the ID could appear in different positions or sometimes not at all. This change removes the channel ID from the message in all cases where it is already present in the structured field. A test run was used to verify that the structured field always appeared in these log calls. Some exceptions remain—for example, calls where the structured field contains a temporary channel ID and the message contains the final ID were left unchanged.

Since most log messages no longer include the channel ID in their text, this commit updates the test logger to output the structured channel ID instead. The ID is truncated for readability. Similarly peer id and payment hash are logged. Co-authored-by: Matt Corallo <[email protected]>

joostjager force-pushed the chan-logging branch 2 times, most recently from 5bae991 to bf104c0 Compare November 11, 2025 13:11

joostjager marked this pull request as ready for review November 11, 2025 13:45

joostjager requested a review from valentinewallace November 11, 2025 13:45

TheBlueMatt reviewed Nov 11, 2025

View reviewed changes

joostjager force-pushed the chan-logging branch from bf104c0 to 9491fa7 Compare November 11, 2025 13:56

joostjager removed the request for review from valentinewallace November 11, 2025 14:00

joostjager force-pushed the chan-logging branch 3 times, most recently from 44dec3f to 2f6b50a Compare November 12, 2025 08:13

joostjager requested a review from TheBlueMatt November 12, 2025 10:29

joostjager force-pushed the chan-logging branch from 2f6b50a to f2483eb Compare November 12, 2025 10:36

joostjager requested a review from valentinewallace November 12, 2025 16:22

TheBlueMatt reviewed Nov 12, 2025

View reviewed changes

joostjager force-pushed the chan-logging branch 4 times, most recently from 61b1780 to 3506f92 Compare November 13, 2025 10:29

joostjager added this to Weekly Goals Nov 13, 2025

joostjager self-assigned this Nov 13, 2025

TheBlueMatt reviewed Nov 13, 2025

View reviewed changes

lightning/src/util/logger.rs Outdated Show resolved Hide resolved

joostjager added 3 commits November 14, 2025 11:44

Implement Display for Record

621f909

As a preparatory step for unifying test log output.

Align log levels

3fc5090

Makes sure that 4 and 5 character log levels line up.

Unify test logger output

30715f5

Now that we have Display implemented on Record, the various test logger implementations can make use of that.

joostjager force-pushed the chan-logging branch from 3506f92 to 2febae0 Compare November 14, 2025 10:50

joostjager requested a review from TheBlueMatt November 14, 2025 10:50

joostjager changed the title ~~Remove redundant channel ID logging~~ Log structured peer, channel and hash fields Nov 14, 2025

joostjager force-pushed the chan-logging branch from 2febae0 to e76d564 Compare November 14, 2025 15:08

joostjager force-pushed the chan-logging branch from e76d564 to 73ea256 Compare November 14, 2025 15:16

valentinewallace reviewed Nov 14, 2025

View reviewed changes

joostjager and others added 2 commits November 14, 2025 20:09

joostjager force-pushed the chan-logging branch from 73ea256 to e069743 Compare November 14, 2025 19:16

joostjager mentioned this pull request Nov 14, 2025

Store and check channel data in channel monitors #4218

Draft

Log structured peer, channel and hash fields #4217

Are you sure you want to change the base?

Log structured peer, channel and hash fields #4217

Conversation

joostjager commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheBlueMatt Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joostjager commented Nov 14, 2025

Uh oh!

joostjager commented Nov 14, 2025

Uh oh!

valentinewallace left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

valentinewallace Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joostjager commented Nov 11, 2025 •

edited

Loading

ldk-reviews-bot commented Nov 11, 2025 •

edited

Loading

codecov bot commented Nov 12, 2025 •

edited

Loading

joostjager commented Nov 14, 2025 •

edited

Loading